-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add DocumentationPage component (for Resources / Quickstart page) and make header/sidebar fixed on scroll #662
Conversation
6cbc637
to
f805d34
Compare
e5b2de6
to
b5c1984
Compare
consistency responsive header update header remove chatgpt comments finish up header and fix up other components lint and delete basecard (will be merged in future pr) Add BaseCard and Header Base Card/text components and veiws make header/sidebar fixed, create documentationpage for quickstart/resources
b5c1984
to
e5dc253
Compare
cd31148
to
e3cb539
Compare
@@ -64,7 +64,7 @@ const Rankings: React.FC = () => { | |||
}, [searchQuery, page]); | |||
|
|||
return ( | |||
<div className="mb-20 ml-10 flex w-full flex-col"> | |||
<div className="flex h-full w-full flex-col overflow-auto p-6"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to use padding instead of margins here, otherwise it will overflow the page width because of the combined w-full
and ml-10
e3cb539
to
31d4fdf
Compare
31d4fdf
to
530824d
Compare
530824d
to
3a66853
Compare
@@ -17,6 +17,7 @@ | |||
"react-dom": "^18.2.0", | |||
"react-hook-form": "^7.45.1", | |||
"react-router-dom": "^6.13.0", | |||
"react-markdown": "^8.0.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the future, we may consider switching to marked
or remarkable
, because they are about 1/3 the size of react-markdown.
we'll just need to figure out how to replace <a>
s that link to urls within play.batlecode.org with <Link>
when using those libraries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Left a few comments about future plans for these markdown pages but looks really good! 🎉
@@ -1,7 +1,9 @@ | |||
import React from "react"; | |||
import DocumentationPage from "../components/DocumentationPage"; | |||
import { BC23_QUICKSTART } from "../content/bc23"; | |||
|
|||
const QuickStart: React.FC = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this view going to always bring up bc23
's Quickstart? Or are we planning to use EpisodeContext on this page (e.g. bc22
,bc23
) that indexes and retrieves <episodeId>_QUICKSTART
for the Quickstart
component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, this will be a later change where we'll get the episode id and display that episode's quickstart
@@ -75,7 +75,7 @@ const Register: React.FC = () => { | |||
{ | |||
// TODO: replace this with our custom notification component |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this on our roadmap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup!
import { BC23_RESOURCES } from "../content/bc23"; | ||
import DocumentationPage from "../components/DocumentationPage"; | ||
|
||
const Resources = (): JSX.Element => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto the comment above about this page eventually rendering the appropriate year's resources based on the URL's episodeId
? If that was already the plan then disregard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we'll render the proper year later. that change will likely come in the same pr as the change that adds the actual 2023 / 2024 episode specific text
… make header/sidebar fixed on scroll (#662) Co-authored-by: Serena Li <serena.li.usa@gmail.com>
… make header/sidebar fixed on scroll (#662) Co-authored-by: Serena Li <serena.li.usa@gmail.com>
… make header/sidebar fixed on scroll (#662) Co-authored-by: Serena Li <serena.li.usa@gmail.com>
Adds the
DocumentationPage
component, which takes markdown text and renders it with react-markdown.Also, makes the header and sidebar fixed
Note: This PR does not contain the correct text for the quickstart / resources pages. The text will be added later b/c it's lower priority